Skip to content

Feedback requested - Initial design documents templates#1

Merged
mkhansenbot merged 9 commits intoros-navigation:masterfrom
mkhansenbot:init_docs
Jun 5, 2018
Merged

Feedback requested - Initial design documents templates#1
mkhansenbot merged 9 commits intoros-navigation:masterfrom
mkhansenbot:init_docs

Conversation

@mkhansenbot
Copy link
Copy Markdown
Collaborator

Initial design documentation templates and examples

@mkhansenbot mkhansenbot requested a review from mjeronimo June 4, 2018 21:37
@@ -0,0 +1,18 @@
# Requirement Title
The <navigation system> should be able to <shall> <do something>
Copy link
Copy Markdown

@mjeronimo mjeronimo Jun 5, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think these angle brackets should be escaped. Markdown allows for some inline markup, so these are not being interpreted correctly by the renderer. They don't show up correctly in Visual Studio code, at least. Searching online, it seems you can use a single backslash before the opening angle bracket: \<navigation system>, for example, instead of <navigation system>.

design/README.md Outdated
@@ -0,0 +1,15 @@
# ROS2 Navigation Design documenation
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo: documenation -> documentation

## More details
- I want to be able to write or use my own collision avoidance algorithm without having to re-compile the entire stack from source
- Ideally I can just change out a node using a custom launch file
- This maps to the "Collision avoidance" use case
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd suggest use case names be capitalized: "Collision avoidance" -> "Collision Avoidance"

@@ -0,0 +1,15 @@
# ROS2 Navigation Design documenation
This is where the ROS2 Navigation design documentation is being collected and vetted
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"ROS2 Navigation" here is inconsistent with the previous README.md, which uses "ROS2 navigation"

design/README.md Outdated
@@ -0,0 +1,15 @@
# ROS2 Navigation Design documenation
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inconsistent capitalization. I'd say either "ROS2 Navigation design documentation" or "ROS2 Navigation Design Documentation" Should be consistent on titles.

- Why is this needed?
- What is the expected user interaction?
- What use case does this map to?

Copy link
Copy Markdown

@mjeronimo mjeronimo Jun 5, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest we add: "Are there any non-functional requirements associated with this functional requirement?"

@@ -0,0 +1,16 @@
# Use case Title
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inconsistent capitalization: "Use Case Title" or "Use case title"

@@ -0,0 +1,16 @@
# Use case Title
As a <Developer, Researcher, Technician, etc.> I want the robot to <action> so that <I, the robot, etc.> can <do something important>
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As before, we should escape these angle brackets.

## More details
- Why is this needed?
- What is the expected user interaction?

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest we add: "Are there any non-functional requirements associated with this use case?"

Copy link
Copy Markdown

@mjeronimo mjeronimo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple typos, some inconsistent capitalization, and a few recommendations.

@mkhansenbot
Copy link
Copy Markdown
Collaborator Author

@mjeronimo - I made fixes & changes you suggested. Please review again.

Copy link
Copy Markdown

@mjeronimo mjeronimo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

@mkhansenbot mkhansenbot merged commit 6ba99d6 into ros-navigation:master Jun 5, 2018
@mkhansenbot
Copy link
Copy Markdown
Collaborator Author

Thanks for the review help!

@mjeronimo
Copy link
Copy Markdown

You bet! Looking forward to getting input from the ROS community.

Jakubach added a commit to Jakubach/navigation2 that referenced this pull request Jan 9, 2026
…ll refactorization

Signed-off-by: Jakubach <jakubach@gmail.com>
@alanxuefei alanxuefei mentioned this pull request Apr 8, 2026
8 tasks
@mini-1235 mini-1235 mentioned this pull request Apr 8, 2026
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants